src/tools/__tests__/health.test.ts (1)

88-92: Consider adding an empty-DB test case.

Per project learnings, new tools should include an output schema smoke test covering the empty-DB case. While the current schema validation test passes, adding an explicit test where countRows returns 0 for all tables would strengthen coverage.

📝 Suggested additional test

it('handles empty tables gracefully', async () => {
	vi.mocked(pgQuery).mockImplementation(async (sql: string) => {
		if (sql === 'SELECT 1') return { rows: [{ '?column?': 1 }], rowCount: 1 };
		return { rows: [{ c: 0 }], rowCount: 1 };
	});
	const result = (await callHealth()) as {
		structuredContent?: { status: string; checks: Record<string, { count?: number }> };
	};
	expect(result.structuredContent?.status).toBe('healthy');
	expect(result.structuredContent?.checks.documents.count).toBe(0);
});

Based on learnings: "New tools must include an output schema smoke test covering the empty-DB case."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tools/__tests__/health.test.ts` around lines 88 - 92, Add an explicit
"empty-DB" unit test to src/tools/__tests__/health.test.ts that mocks pgQuery so
countRows returns 0 for tables and the health check still conforms to
HealthCheckOutputSchema; specifically, create a test (e.g., it('handles empty
tables gracefully')) that vi.mocked(pgQuery).mockImplementation to return the
SELECT 1 probe response and zero counts for other queries, call callHealth(),
assert result.structuredContent.status is 'healthy' and that
result.structuredContent.checks.documents.count === 0, and ensure the output
still validates against HealthCheckOutputSchema.

src/tools/health.ts (1)

22-25: Consider using a string literal union type for table safety.

While currently safe (all callers pass hardcoded literals), adding type-level constraints prevents accidental misuse during future refactoring.

🛡️ Optional defensive typing

+type HealthTable =
+	| 'documents'
+	| 'chunks'
+	| 'memory_entities'
+	| 'conversation_sessions'
+	| 'proactive_insights';
+
-async function countRows(table: string): Promise<number> {
+async function countRows(table: HealthTable): Promise<number> {
 	const result = await pgQuery<{ c: number }>(`SELECT COUNT(*)::int AS c FROM ${table}`);
 	return result.rows[0]?.c ?? 0;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tools/health.ts` around lines 22 - 25, Replace the untyped string
parameter on countRows(table: string) with a string literal union type listing
the allowed table names (e.g., type TableName = '...') and change the signature
to countRows(table: TableName): Promise<number>; update any callers if they rely
on non-literal values and export the TableName type if other modules need it.
This constrains usages at the type level while keeping the function body
(pgQuery and SQL string) unchanged, preventing accidental future refactor bugs.

src/tools/__tests__/health.test.ts (1)

39-113: Consider adding a test for the unhealthy status.

The test suite covers healthy and degraded statuses but not the unhealthy case where all checks fail. Since this is a diagnostic tool that will be "the first call when other tools are failing," verifying the all-failed scenario adds confidence.

💡 Example test for unhealthy status

it('returns unhealthy when all checks fail', async () => {
	vi.mocked(pgQuery).mockRejectedValue(new Error('connection refused'));
	const result = (await callHealth()) as {
		structuredContent?: { status: string };
	};
	expect(result.structuredContent?.status).toBe('unhealthy');
});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tools/__tests__/health.test.ts` around lines 39 - 113, Add a test that
verifies the "unhealthy" status by forcing all subsystem checks to fail (mock
pgQuery used by createHandler/callHealth to reject or throw), call
createHandler() and assert that the returned structuredContent.status equals
'unhealthy' (and optionally validate the structure with
HealthCheckOutputSchema/z.object). Specifically, add a new it('returns unhealthy
when all checks fail', ...) that uses vi.mocked(pgQuery).mockRejectedValue(new
Error('connection refused')) (or similar), invokes callHealth(), and expects
result.structuredContent?.status toBe('unhealthy').

src/tools/health.ts (1)

29-32: SQL injection risk via direct table name interpolation.

While TableName is a string literal union type that restricts valid values at compile time, the table parameter is interpolated directly into the SQL string. If this function were ever called with a value that bypasses TypeScript checking (e.g., from a type assertion or any), it could enable SQL injection.

Consider using a safelist approach for defense-in-depth:

🛡️ Proposed fix with runtime validation

+const ALLOWED_TABLES: ReadonlySet<TableName> = new Set([
+	'documents',
+	'chunks',
+	'memory_entities',
+	'conversation_sessions',
+	'proactive_insights',
+]);
+
 async function countRows(table: TableName): Promise<number> {
+	if (!ALLOWED_TABLES.has(table)) {
+		throw new Error(`Invalid table name: ${table}`);
+	}
 	const result = await pgQuery<{ c: number }>(`SELECT COUNT(*)::int AS c FROM ${table}`);
 	return result.rows[0]?.c ?? 0;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tools/health.ts` around lines 29 - 32, The countRows function currently
interpolates the table identifier directly into SQL, creating a potential SQL
injection vector if TypeScript checks are bypassed; change countRows to perform
runtime validation against a safelist of allowed table names (e.g., a const Map
or object keyed by TableName -> actual table identifier) and use the mapped,
validated identifier in the query, throwing an error for unknown values;
reference the countRows function, the TableName type, and pgQuery when
implementing this runtime whitelist check to ensure only approved identifiers
are used.